Fix generated keys retrieval for PostgreSQL SINGLE tables (#37485)#38024
Open
aviralgarg05 wants to merge 4 commits intoapache:masterfrom
Open
Fix generated keys retrieval for PostgreSQL SINGLE tables (#37485)#38024aviralgarg05 wants to merge 4 commits intoapache:masterfrom
aviralgarg05 wants to merge 4 commits intoapache:masterfrom
Conversation
When inserting into a SINGLE table with @GeneratedValue(IDENTITY) on PostgreSQL, getGeneratedKeys() threw NumberFormatException because it always read column index 1 from the underlying ResultSet. PostgreSQL JDBC driver returns all columns via RETURNING *, so column 1 may not be the generated key column. Fix: use the generated key column name from GeneratedKeyContext (when available) to read the correct column via resultSet.getObject(columnName) instead of blindly reading resultSet.getObject(1). Closes apache#37485
terrymanu
requested changes
Feb 15, 2026
Member
terrymanu
left a comment
terrymanu
left a comment
There was a problem hiding this comment.
Decision
- Merge Verdict: Not Mergeable
- Reviewed Scope: latest PR revision (bd4ecca, 2026-02-12), changed files
jdbc/src/main/java/org/apache/shardingsphere/driver/jdbc/core/statement/ShardingSpherePreparedStatement.java,
jdbc/src/main/java/org/apache/shardingsphere/driver/jdbc/core/statement/ShardingSphereStatement.java,
.github/workflows/e2e-agent.yml, related issue #37485, and PR checks page. - Not Reviewed Scope: full GitHub Actions job logs for every failed matrix item, runtime reproduction on real MySQL/PostgreSQL instances.
- Need Expert Review: JDBC/dialect maintainer review is needed for generated-key behavior across PostgreSQL/MySQL/MariaDB drivers.
Positive Feedback
- The fix direction is correct for the reported PostgreSQL SINGLE-table trigger path: fallback key extraction now uses a named column instead of always index 1, which targets the issue chain from #37485.
Major Issues
- High: cross-dialect compatibility regression risk in generated-key fallback.
Symptom: both fallback paths now prefer resultSet.getObject(columnName) in
jdbc/src/main/java/org/apache/shardingsphere/driver/jdbc/core/statement/ShardingSpherePreparedStatement.java:330 and
jdbc/src/main/java/org/apache/shardingsphere/driver/jdbc/core/statement/ShardingSpherePreparedStatement.java:334, plus
jdbc/src/main/java/org/apache/shardingsphere/driver/jdbc/core/statement/ShardingSphereStatement.java:374 and
jdbc/src/main/java/org/apache/shardingsphere/driver/jdbc/core/statement/ShardingSphereStatement.java:379.
Risk: MySQL dialect explicitly defines generated-key label "GENERATED_KEY" at
database/connector/dialect/mysql/src/main/java/org/apache/shardingsphere/database/connector/mysql/metadata/database/MySQLDatabaseMetaData.java:84
(and MariaDB delegates at database/connector/dialect/mariadb/src/main/java/org/apache/shardingsphere/database/connector/mariadb/metadata/database/MariaDBDatabaseMetaData.java:85), so using logical key column names may break drivers that expose only generated-key
labels in getGeneratedKeys() result sets.
Recommended action: make retrieval dialect-aware (use effective generated-key label or safe fallback to index 1) and add regression tests for PostgreSQL + MySQL/MariaDB. - High: test evidence is insufficient for changed public behavior.
Symptom: PR changes two production classes but no src/test files; existing test coverage for prepared statement is a single scenario unrelated to this branch at
jdbc/src/test/java/org/apache/shardingsphere/driver/jdbc/core/statement/ShardingSpherePreparedStatementTest.java:52.
Risk: root-cause path and adjacent regression paths are not proven.
Recommended action: add dedicated tests mapping one-to-one to changed paths in both getGeneratedKeys() methods (column-name path, index fallback path, MySQL-style generated-key label path). - Medium: validation signal is incomplete/unresolved.
Symptom: PR checks page currently shows failed jobs (for example, E2E - Agent / Build E2E Image, plus multiple E2E matrix failures).
Risk: unresolved CI reduces merge confidence and conflicts with submission expectations in CODE_OF_CONDUCT.md:18 and coverage expectations in CODE_OF_CONDUCT.md:20.
Recommended action: rerun/fix failing checks or provide maintainer-confirmed flaky classification with fresh green evidence for this commit.
Newly Introduced Issues
- The current fallback implementation can introduce new failures on dialects that do not expose generated keys under the logical column name (potentially MySQL/MariaDB SINGLE-table flows that previously worked via index-based extraction).
Unrelated Changes
- Please roll back or split out .github/workflows/e2e-agent.yml:77 and .github/workflows/e2e-agent.yml:91 (commit bd4ecca “ci: validate e2e-agent image archives”).
- This CI workflow scope is unrelated to issue #37485 (PostgreSQL generated-key retrieval bug), so it should not be bundled in this bug-fix PR.
Next-Step Suggestions
- Remove/split the workflow commit from this PR.
- Adjust fallback key extraction to be dialect-safe, not only column-name-based.
- Add targeted unit tests for both modified public methods and both retrieval branches.
- Re-run checks and post fresh CI evidence for the latest head commit.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
When inserting into a SINGLE table with
@GeneratedValue(IDENTITY)on PostgreSQL,getGeneratedKeys()threwNumberFormatExceptionbecause it always read column index 1 from the underlyingResultSet. The PostgreSQL JDBC driver returns all columns viaRETURNING *whenRETURN_GENERATED_KEYSis used, so column 1 may not be the generated key column (it could be a string column likeevent).Fix
Used the generated key column name from
GeneratedKeyContext(when available) to read the correct column viaresultSet.getObject(columnName)instead of blindly readingresultSet.getObject(1).Fixes #37485.
Changes proposed in this pull request
ShardingSpherePreparedStatement.getGeneratedKeys()to derivecolumnNameearlier and use it for selection in the fallback path.ShardingSphereStatement.getGeneratedKeys()to derivecolumnNameearlier and use it for selection in the fallback path.resultSet.getObject(1)remains ifcolumnNameis not available.Before committing this PR, I'm sure that I have checked the following options:
type: bug,db: PostgreSQL,in: JDBC)./mvnw clean install -B -T1C -Dmaven.javadoc.skip -Dmaven.jacoco.skip -e(Verified with./mvnw -pl jdbc test- 1148 tests passed).GeneratedKeysResultSetTestandShardingSpherePreparedStatementTestcover these paths and passed).